Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Un-exclude interm_tesscut_dss_overlay #77

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eteq
Copy link
Collaborator

@eteq eteq commented Mar 20, 2019

This is a test PR to see if taking interm_tesscut_dss_overlay out of the exclude list now allows the tests to pass. The origin of this problem was initially confusing, and at the very least maybe the update log here will help diagnose it? We shall see what the tests show...

@eteq eteq requested a review from astro-josh as a code owner March 20, 2019 06:17
@astro-josh
Copy link
Contributor

seems convert got caught up on align_multiple_visits

@pllim
Copy link
Contributor

pllim commented Mar 20, 2019

@samanthalh , can you think of why the PNG to be renamed is non-existent in the DrizzlePac notebook? Please advise, thanks.

FileNotFoundErrorTraceback (most recent call last)
<ipython-input-7-7a75d58f34ed> in <module>
      1 #Give the 'fit residual plots' a unique name for comparison with other tests.
      2 
----> 3 os.rename('residuals_j9irw5kaq_flc.png','residuals_j9irw5kaq_flc_default.png')
      4 Image(filename='residuals_j9irw5kaq_flc_default.png', width=500, height=600)

FileNotFoundError: [Errno 2] No such file or directory: 'residuals_j9irw5kaq_flc.png' -> 'residuals_j9irw5kaq_flc_default.png'

@pllim

This comment has been minimized.

@ceb8
Copy link
Collaborator

ceb8 commented Mar 20, 2019

@pllim Glad to hear! Was going crazy not being able to find that code in the notebook.

@samanthalh
Copy link
Contributor

@pllim, that PNG should be output by TweakReg in a previous cell. So either TweakReg isn't running for some reason or it's saving that file in a strange place that then cannot be found by the os command.

Nothing has changed about the notebook, so I can only guess that this is a difference between Travis and this new ci/circleci that is apparently now being used. I have no idea about why that would matter, though, so you'll have to ask @eteq or one of the other repo maintainers.

@pllim
Copy link
Contributor

pllim commented Mar 20, 2019

Talking about TweakReg, @mcara , any ideas?

@samanthalh
Copy link
Contributor

@pllim, I really do think that it's a GitHub issue and not something going wrong with TweakReg, although of course I'd be glad to hear @mcara weigh in on the issue.

@catherine-martlin
Copy link
Contributor

I remember that we had some issues with paths that would work when individuals ran the notebook but failed when the CI tried to when we were submitting these the first time around - is it possible that os.rename() didn't rename the file? Maybe place a test line in the notebook about whether 'residuals_j9irw5kaq_flc.png' exists would help suss out whether it's TweakReg or the os.rename() function causing the issue?

If it is an issue with os.rename() then it may be necessary to add in some of the absolute path finagling we did before.

A second idea may be something similar to what we struggled with in #34 where we had to specify the input list for the output files of TweakReg to be named as expected?

@pllim
Copy link
Contributor

pllim commented Mar 20, 2019

According to @mcara, /root/repo/notebooks/DrizzlePac/align_multiple_visits/tweakreg.log should give more ideas on what happened. @obviousrebel , how do I get that artifact out of CircleCI?

@mcara
Copy link
Member

mcara commented Mar 20, 2019

I do not see any obvious issues with tweakreg itself. One thing to check is to look at the log file and see which of the input images was chosen as a reference image. IF j9irw5kaq_flc.fits was chosen as a reference image - this could cause the plots for it to not be created. By default tweakreg chooses the first input image as a reference image. So, could it be that on your test machine the input file list is in a different order compared to other machines? Just an idea to check.

@mcara
Copy link
Member

mcara commented Mar 20, 2019

Try sorting input file list to ensure the same order. See if this helps improve consistency.

@pllim
Copy link
Contributor

pllim commented Mar 20, 2019

@astro-josh
Copy link
Contributor

astro-josh commented Mar 20, 2019

@pllim Artifacts can be viewed here. At the top of the page you can navigate through the directories. In that folder, there are two pngs named residuals_j9irw3fwq_flc.png and residuals_j9irw4b1q_flc.png.

tweakreg log file

@pllim
Copy link
Contributor

pllim commented Mar 20, 2019

Bingo, Mihai wins: reference image 'j9irw5kaq_flc.fits'

@pllim
Copy link
Contributor

pllim commented Mar 20, 2019

I am trying to fix the TweakReg thingy over at #78.

@pllim
Copy link
Contributor

pllim commented Mar 21, 2019

@eteq , if you rebase, the DrizzlePac error will not bother this PR anymore.

@pllim
Copy link
Contributor

pllim commented Sep 19, 2019

This probably need a rebase and see if CI would still pass.

@pllim pllim removed the request for review from jbcurtin September 19, 2019 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants